Optimize eoRanking: Add caching and index vector#80
Conversation
nojhan
left a comment
nojhan
left a comment
There was a problem hiding this comment.
Thanks for the contribution! This is a neat feature, which we must include in Paradiseo.
However, adding a cache system rely on the idea that the instantiated operator is always used in the same context. In the case of automated design, this is not always the case. Thus, I think this class should either:
- be a new class, like
eoRankingCached, - have a configurable cache, with the cache disabled by default.
I would also like to have more comments, given that the system is not immediately obvious to the reader. Something like starting from the PR message and extend a bit, in the class' header, perhaps?
Finally, a simple test ensuring that both implementations produces the same result would be a convincing check that everything is perfectly working.
|
Thank you for your review and suggestions. I've implemented the requested changes:
|
nojhan
left a comment
nojhan
left a comment
There was a problem hiding this comment.
Impressive work!
Apart from the very minor comments, I think it would be better if we had those two classes in two separate files. I know it's not always the case in all the old codebase, but I think that would really help discoverability, for users who look at files directly.
Also, the indentation seems to be inconsistent, with sometime two spaces, sometime four. I think four is more or less the standard in Paradiseo (even if we have mixed styles…).
|
Thank you for your review and valuable feedback. I have implemented both of your requested changes:
Additionally, the newly added test cases confirm that all assertions are functioning as intended. |
nojhan
left a comment
nojhan
left a comment
There was a problem hiding this comment.
Almost perfect! I'm just nitpicking about asserts, but once fixed, I will merge. I tested locally, and everything merges well and passes tests.
|
Also, don't forget to "resolve conversations" that are fixed in further commits. |
nojhan
left a comment
nojhan
left a comment
There was a problem hiding this comment.
Awesome! Thanks for the contribution!
|
Thanks for the detailed feedback and suggestions! |
|
No problem. I'm looking forward for the next contribution ;-) And I hope you find Paradiseo useful. |
2 participants
This PR improves the performance of the eoRanking class by introducing two key enhancements:
Cached Coefficients:
Precomputes and stores intermediate constants (e.g., alpha, beta, gamma).
These are recomputed only when the population size changes, improving runtime efficiency for repeated evaluations.
Index Vector:
Replaces the costly lookfor loop with a std::vector for O(1) lookup of individual indices.
This avoids potential runtime errors and simplifies the ranking logic.
Fully preserves compatibility with the EO framework (eoPerf2Worth, eoPop).